fix: resolve Next.js build errors and optimize embeddings loading#594
Conversation
- Marked API routes as force-dynamic to fix prerendering failures. - Refactored landing page for static generation with client-side UUIDs. - Updated auth logic to await cookies() for Next.js 16. - Optimized embeddings index loading with non-blocking singleton pattern. - Suppressed Turbopack NFT tracing warnings for dynamic paths. Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoFix Next.js build errors and optimize embeddings loading
WalkthroughsDescription• Marked API routes as force-dynamic to fix Next.js prerendering failures • Refactored landing page to use placeholder chat ID with client-side UUID generation • Optimized embeddings index loading with async promise-based singleton pattern • Updated auth cookie handling to await cookies() for Next.js 16 compatibility • Added Turbopack ignore comments for dynamic path resolutions Diagramflowchart LR
A["API Routes"] -- "force-dynamic export" --> B["Prerendering Fixed"]
C["Landing Page"] -- "placeholder ID + client UUID" --> D["Static Generation"]
E["Embeddings Index"] -- "async singleton promise" --> F["Non-blocking I/O"]
G["Auth Cookies"] -- "await cookies()" --> H["Next.js 16 Compatible"]
I["Dynamic Paths"] -- "turbopackIgnore comments" --> J["NFT Tracing Resolved"]
File Changes1. app/api/chat/route.ts
|
Code Review by Qodo
1. Stray page file compiled
|
| import { Chat } from '@/components/chat' | ||
| import { AI } from './actions' | ||
|
|
There was a problem hiding this comment.
1. Stray page file compiled 🐞 Bug ≡ Correctness
The new root-level app_page.tsx is included by tsconfig.json (**/*.tsx) and imports ./actions, which resolves relative to repo root and does not match the actual app/actions.tsx location. This can break next build/typecheck with a module resolution error.
Agent Prompt
### Issue description
A root-level `app_page.tsx` is being typechecked (tsconfig includes `**/*.tsx`) and it imports `./actions`, which is not a valid path from repo root (actions live under `app/actions.tsx`). This can fail the TypeScript/Next build.
### Issue Context
This file appears to be a duplicate of `app/page.tsx`, but its location makes its relative imports incorrect.
### Fix Focus Areas
- tsconfig.json[31-37]
- app_page.tsx[1-16]
- app/page.tsx[1-16]
### Suggested fix
- Delete `app_page.tsx` if it was added accidentally, **or** move it under `app/` as `app/page.tsx` (and ensure only one page exists).
- If you intentionally need this file at repo root, fix imports to use absolute aliases (e.g. `import { AI } from '@/app/actions'`) and ensure it won’t be treated as a route unintentionally.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export function Chat({ id: initialId }: ChatProps) { | ||
| const [id, setId] = useState(initialId || '') | ||
|
|
||
| useEffect(() => { | ||
| if (!id || id === 'new-chat') { | ||
| setId(crypto.randomUUID()) | ||
| } | ||
| }, [id]) |
There was a problem hiding this comment.
2. Chat ids diverge client/server 🐞 Bug ≡ Correctness
On the landing page, AI state initializes with chatId: 'new-chat' while the Chat component
generates a different UUID in local state and updates the URL to /search/{uuid}. Server
persistence uses the AI state’s chatId, and chat-db.saveChat deletes 'new-chat' to let the DB
generate another UUID, causing chat fragmentation and DB writes (e.g., updateDrawingContext) to
target a non-existent chatId.
Agent Prompt
### Issue description
The landing page uses a placeholder AI `chatId` (`'new-chat'`), while the client `Chat` generates a different UUID and updates the URL. Server persistence uses the AI state's `chatId`, and the DB layer deletes `'new-chat'` and generates yet another UUID. This results in inconsistent IDs between URL/client/server/DB and can fragment chats or break DB writes.
### Issue Context
- Client `Chat` generates an id and rewrites the URL.
- AI state still holds `'new-chat'` unless explicitly updated.
- `lib/actions/chat.ts` persists using `chat.id` from AI state.
- `chat-db.saveChat` deletes `'new-chat'` so DB generates a different UUID.
- `updateDrawingContext(chatId, ...)` writes messages keyed by the client id, but `messages.chatId` must reference an existing `chats.id`.
### Fix Focus Areas
- app/page.tsx[8-13]
- components/chat.tsx[29-36]
- components/chat.tsx[80-84]
- app/actions.tsx[574-659]
- lib/actions/chat.ts[90-147]
- lib/actions/chat-db.ts[83-113]
### Suggested fix
Pick a single source of truth for `chatId` and propagate it consistently:
1) **Client-generated UUID becomes authoritative**:
- In `Chat`, use `useAIState` setter (or another action) to set AI state's `chatId` once the UUID is generated.
- Stop deleting `'new-chat'` in `chat-db.saveChat` once AI state is updated; instead, ensure DB `chats.id` is created using that UUID.
2) **Or server-generated UUID becomes authoritative**:
- Create the chat on the server before any message/context writes and return the UUID to the client; update URL/client state with that UUID.
Also ensure `updateDrawingContext` only runs after a `chats` row exists for the chosen `chatId` (or create it first).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const body = await request.json(); | ||
|
|
||
| // Example: Distinguish between creating a new chat vs. adding a message to existing chat | ||
| // The actual structure of `body` would depend on client-side implementation. | ||
| // Let's assume a simple case: creating a new chat with an initial message. | ||
| const { title, initialMessageContent, role = 'user' } = body; | ||
| const { title, initialMessageContent, role = 'user', chatId } = body; | ||
|
|
||
| if (!initialMessageContent) { | ||
| return NextResponse.json({ error: 'Initial message content is required' }, { status: 400 }); | ||
| } | ||
|
|
||
| const newChatData: NewChat = { | ||
| // id: generateUUID(), // Drizzle schema now has defaultRandom for UUIDs | ||
| id: chatId, | ||
| userId: userId, | ||
| title: title || 'New Chat', // Default title if not provided | ||
| // createdAt: new Date(), // Handled by defaultNow() in schema | ||
| visibility: 'private', // Default visibility | ||
| title: title || 'New Chat', | ||
| visibility: 'private', | ||
| }; |
There was a problem hiding this comment.
3. Chat updates lack ownership check 🐞 Bug ⛨ Security
POST /api/chat now accepts a client-supplied chatId and passes it to chat-db.saveChat, but saveChat selects/updates chats by chats.id only (no userId constraint). If a chat UUID is guessed/leaked, this allows cross-user chat mutation (title/visibility updates and message insertion).
Agent Prompt
### Issue description
`/api/chat` accepts a client-provided `chatId`, and `chat-db.saveChat` updates chats by `id` only. This can allow a user to write into another user's chat if they know the UUID.
### Issue Context
- `app/api/chat/route.ts` takes `chatId` from request JSON and sets it as `NewChat.id`.
- `lib/actions/chat-db.ts::saveChat` checks existence and updates using `where(eq(chats.id, chatId))` without scoping to `userId`.
### Fix Focus Areas
- app/api/chat/route.ts[13-25]
- lib/actions/chat-db.ts[95-113]
- lib/db/schema.ts[30-40]
### Suggested fix
- In `chat-db.saveChat`, scope all read/update operations by both `chats.id` and `chats.userId`:
- `where(and(eq(chats.id, chatId), eq(chats.userId, chatData.userId)))`
- Apply the same constraint to `update(chats)...where(...)`.
- If a chat with the requested `id` exists but belongs to a different user, reject (throw/return null) rather than updating.
- In `POST /api/chat`, validate `chatId` (UUID format) and decide whether you truly want to allow client-chosen IDs; if not required, ignore it and let the DB generate the ID server-side.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This PR addresses several critical build and performance issues encountered during the Next.js upgrade:
cookies(),request.url, orcrypto.randomUUID()were failing during build. I've marked the API routes asforce-dynamicand refactored the landing page to be static by moving ID generation to the client.lib/auth/get-current-user.tstoawait cookies(), which is now required.PR created automatically by Jules for task 8901716965629377828 started by @ngoiyaeric